Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass multiprocessing parameters through the shell #952

Merged
merged 9 commits into from
Jul 27, 2020

Conversation

dcmaddix
Copy link
Contributor

Summary

  • Exposed the multiprocessing parameters num_workers, num_prefetch, shuffle_buffer_length as hyperparameter keys through the shell
  • Updated run_train method to read these keys in from the hyper-parameters and set their values
  • For num_workers we set to be the minimum of inputed value if set and if not input default 4 and the sort of the number of cpus
  • Added logging to log these parameter values inparallelized_loader.py
  • Added logging to whether the dataset was cached or not and its type, i.e. ListDataset and FileDataset, respectively

Testing

  • Add MQCNNEstimator as forecaster_type to test_train_shell to extend the test for trainable algorithms unlike the current MeanPredictor
  • Passed the three multiprocessing parameters as hyper-parameters and used the smoke test with the MQCNNEstimator

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@dcmaddix
Copy link
Contributor Author

dcmaddix commented Jul 26, 2020

There is a timeout test error of 30s for the additional test I added with the MQCNNEstimator. It takes longer than that to run. Is there a way to increase the test timeout?

Danielle Robinson and others added 2 commits July 27, 2020 08:31
AaronSpieler
AaronSpieler previously approved these changes Jul 27, 2020
Copy link
Contributor

@AaronSpieler AaronSpieler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thx!

…er as 0 so the user will need to turn multiprocessing explicilty on
@dcmaddix dcmaddix merged commit 42bee73 into awslabs:master Jul 27, 2020
@dcmaddix dcmaddix deleted the mp-param-shell branch July 27, 2020 23:13
kashif pushed a commit to kashif/gluon-ts that referenced this pull request Oct 10, 2020
* Passing num_workers, num_shuffle_batches and is_cached through the shell

* Pass num_workers, num_prefetch and shuffle_buffer_length through the shell

* Fix black

* Fix spacing

* Revert back to shuffle_buffer_length

* Decreasing number of training epochs in the smoke test for MQCNN so that it finishes within the timeout limit

* Default to num_workers 2.

* Setting default num_workers = None to be set in the parallelized_loader as 0 so the user will need to turn multiprocessing explicilty on

Co-authored-by: Danielle Robinson <dmmaddix@amazon.com>
Co-authored-by: Aaron Spieler <25365592+AaronSpieler@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants